Skip to content

Add support for string config fields - #5

Merged
robin-nitrokey merged 1 commit into
trussed-dev:mainfrom
ABuljko:config-string
Aug 17, 2026
Merged

Add support for string config fields#5
robin-nitrokey merged 1 commit into
trussed-dev:mainfrom
ABuljko:config-string

Conversation

@ABuljko

@ABuljko ABuljko commented Aug 11, 2026

Copy link
Copy Markdown
Contributor
  • Add FieldType::String and ConfigValueMut::String(&mut StringView)
  • Put both after U8, so Bool=0 and U8=1 stay the same
  • Keep FieldType::String a unit variant: the max length comes from the config struct, and values that are too long give DataTooLong
  • Take a StringView, so any String<N> works: hold a String<32> and pass as_mut_view()
  • Reject control characters, line and paragraph separators, and bidirectional embeddings, overrides and isolates with InvalidValue; the directional marks are still allowed
  • Check the length before clear(), so a rejected value keeps the old one
  • Enable the serde feature of heapless, needed for StringView's Serialize impl
  • Add tests for getting and setting a string field, both rejections, multi-byte values, and the field type ids

@robin-nitrokey robin-nitrokey left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Just one question regarding the forbidden characters:

Comment thread src/config.rs Outdated
@ABuljko
ABuljko marked this pull request as ready for review August 11, 2026 17:11

@robin-nitrokey robin-nitrokey left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Please remove the outdated comment and add an entry to the changelog.

Comment thread src/config.rs Outdated

@robin-nitrokey robin-nitrokey left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! To do before merging:

  • squash commits
  • create draft PR for nitrokey-3-firmware to show how this is used

@ABuljko

ABuljko commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

Should i temporarily link the nitrokey-3-firmware PR to my branch of the admin-app instead of main so that it can be tested properly

@robin-nitrokey

Copy link
Copy Markdown
Member

Yes, you can use a patch.crates-io section in Cargo.toml and point it to your branch or this PR.

- Add FieldType::String and ConfigValueMut::String, backed by a StringView so
  any String<N> can be used
- Put both after U8 so the existing type ids stay the same
- Take the max length from the config struct; too long gives DataTooLong
- Check the length before clear() so a rejected value keeps the old one
- Store arbitrary UTF-8; the client sanitizes the value before displaying it
- Enable the serde feature of heapless, needed for StringView's Serialize impl
@robin-nitrokey
robin-nitrokey merged commit f0a685b into trussed-dev:main Aug 17, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants